Skip to content

fix(agent): skip empty switch conditions#15691

Merged
yingfeng merged 1 commit into
infiniflow:mainfrom
he-yufeng:fix/switch-empty-condition
Jun 5, 2026
Merged

fix(agent): skip empty switch conditions#15691
yingfeng merged 1 commit into
infiniflow:mainfrom
he-yufeng:fix/switch-empty-condition

Conversation

@he-yufeng
Copy link
Copy Markdown
Contributor

What

  • make Switch ignore conditions that have no evaluable items
  • add a regression for blank cpn_id items falling through to the else branch
  • keep the existing non-empty and condition behavior covered

Fixes #15643.

Verified

  • python -m py_compile agent\component\switch.py test\unit_test\agent\component\test_switch.py
  • python -m pytest test\unit_test\agent\component\test_switch.py -q -> 2 passed
  • python -m ruff check agent\component\switch.py test\unit_test\agent\component\test_switch.py
  • git diff --check

I also checked python -m ruff format --check on the touched files. It would reformat pre-existing style in agent/component/switch.py beyond this bug fix, so I kept the patch scoped instead of reformatting the whole file.

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. 🌈 python Pull requests that update Python code 🐞 bug Something isn't working, pull request that fix bug. 🧪 test Pull requests that update test cases. labels Jun 5, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 5, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Switch._invoke now checks res and all(res) instead of all(res) to prevent empty conditions from matching. New tests verify that conditions with no evaluable items correctly fall through to the else target, and non-empty conditions still route as expected.

Changes

Switch empty condition evaluation fix

Layer / File(s) Summary
Fix empty condition evaluation bug
agent/component/switch.py
Switch._invoke checks res and all(res) to require at least one evaluable result, preventing empty conditions from incorrectly routing via all([]) being True.
Test coverage for switch condition matching
test/unit_test/agent/component/test_switch.py
New test module with _Canvas test double and _switch helper; two test cases verify empty conditions fall through to else, and matching non-empty conditions route to their target.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 An empty list once tricked the switch so sly,
With all([]) waving its deceptive high—
No more! One guard now checks the path with care,
Routes flow to else when conditions aren't there. 🌿

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes details on what changed and verification steps, but does not follow the provided template structure with required sections like 'What problem does this PR solve?' and 'Type of change'. Add a 'What problem does this PR solve?' section and mark the appropriate 'Type of change' checkbox (Bug Fix) from the template to match repository standards.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing the Switch component to skip empty conditions, which is the primary objective of this PR.
Linked Issues check ✅ Passed The PR changes fully implement the fix for issue #15643: the code change prevents all([]) from evaluating as True by adding 'res and' check, and tests cover both empty condition fallthrough and non-empty condition matching scenarios.
Out of Scope Changes check ✅ Passed All changes are directly within scope: the Switch._invoke bug fix addresses issue #15643, the new test file verifies the fix with appropriate test cases covering the regression and existing behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/unit_test/agent/component/test_switch.py`:
- Around line 26-41: Add the appropriate pytest priority marker to the test
function test_switch_empty_condition_falls_through_to_else (and the other test
in the same file referenced at lines 44-59) by decorating each test with
`@pytest.mark.p1` (or the correct priority per test suite rules); ensure pytest is
imported at the top of test_switch.py (add "import pytest" if absent) so the
decorator resolves.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d9194356-7683-4e84-b082-8356e0aed2d7

📥 Commits

Reviewing files that changed from the base of the PR and between f78ef32 and a24de35.

📒 Files selected for processing (2)
  • agent/component/switch.py
  • test/unit_test/agent/component/test_switch.py

Comment on lines +26 to +41
def test_switch_empty_condition_falls_through_to_else():
param = SwitchParam()
param.conditions = [
{
"logical_operator": "and",
"items": [{"cpn_id": "", "operator": "=", "value": "yes"}],
"to": ["case_target"],
}
]
param.end_cpn_ids = ["else_target"]

cpn = _switch(param)
cpn._invoke()

assert cpn.output("_next") == ["else_target"]
assert cpn.output("next") == ["else_target"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add pytest priority markers to test functions.

Both test functions are missing priority markers (@pytest.mark.p1, @pytest.mark.p2, or @pytest.mark.p3). As per coding guidelines, all tests in test/**/*.py should use pytest priority markers.

📝 Proposed fix to add priority markers
+import pytest
+

 from agent.component.switch import Switch, SwitchParam


 class _Canvas:
+@pytest.mark.p1
 def test_switch_empty_condition_falls_through_to_else():
     param = SwitchParam()
+@pytest.mark.p1
 def test_switch_non_empty_and_condition_still_matches():
     param = SwitchParam()

Also applies to: 44-59

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/unit_test/agent/component/test_switch.py` around lines 26 - 41, Add the
appropriate pytest priority marker to the test function
test_switch_empty_condition_falls_through_to_else (and the other test in the
same file referenced at lines 44-59) by decorating each test with
`@pytest.mark.p1` (or the correct priority per test suite rules); ensure pytest is
imported at the top of test_switch.py (add "import pytest" if absent) so the
decorator resolves.

@yingfeng yingfeng added the ci Continue Integration label Jun 5, 2026
@yingfeng yingfeng marked this pull request as draft June 5, 2026 07:13
@yingfeng yingfeng marked this pull request as ready for review June 5, 2026 07:13
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.16%. Comparing base (f78ef32) to head (a24de35).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #15691   +/-   ##
=======================================
  Coverage   93.16%   93.16%           
=======================================
  Files          10       10           
  Lines         717      717           
  Branches      118      118           
=======================================
  Hits          668      668           
  Misses         29       29           
  Partials       20       20           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yingfeng yingfeng merged commit 6cba5a5 into infiniflow:main Jun 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug Something isn't working, pull request that fix bug. ci Continue Integration 🌈 python Pull requests that update Python code size:XS This PR changes 0-9 lines, ignoring generated files. 🧪 test Pull requests that update test cases.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Switch matches an empty condition because all([]) is True

2 participants